Skip to content

Alphabet#456

Merged
james-d-mitchell merged 5 commits into
libsemigroups:mainfrom
james-d-mitchell:alphabet
Jun 19, 2026
Merged

Alphabet#456
james-d-mitchell merged 5 commits into
libsemigroups:mainfrom
james-d-mitchell:alphabet

Conversation

@james-d-mitchell

Copy link
Copy Markdown
Member

No description provided.

@james-d-mitchell james-d-mitchell added the libsemigroups-feature-not-yet-supported Label for issues and PRs related to features of libsemigroups not yet supported here label Jun 18, 2026

@Joseph-Edwards Joseph-Edwards left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other than some small comments, this looks good to me! Thanks @james-d-mitchell

Comment thread src/alphabet.cpp
Comment thread tests/conftest.py
Comment on lines +11 to +13
rg = ReportGuard(False)
yield
del rg

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
rg = ReportGuard(False)
yield
del rg
return ReportGuard(False)

?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whilst looking into this, I realised our doctest reporting suppression hasn't been working. I think the fix is

 # Python code that is treated like it were put in a testsetup directive for
 # every file that is tested, and for every group.
 doctest_global_setup = """from libsemigroups_pybind11 import ReportGuard
-ReportGuard(False)"""
+rg = ReportGuard(False)"""

in line 192 of docs/source/conf.py.

If you make the pytest change above, please could you make the doctest one too. Thank you

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
rg = ReportGuard(False)
yield
del rg
return ReportGuard(False)

?

I don't think this works, due to the lifetime of the ReportGuard ending before the tests are run. I might be wrong, but I tried a number of different things here and this one works. The del is unnecessary but stops the linter from complaining about rg being unused so I left it in

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whilst looking into this, I realised our doctest reporting suppression hasn't been working. I think the fix is

 # Python code that is treated like it were put in a testsetup directive for

 # every file that is tested, and for every group.

 doctest_global_setup = """from libsemigroups_pybind11 import ReportGuard

-ReportGuard(False)"""

+rg = ReportGuard(False)"""

in line 192 of docs/source/conf.py.

If you make the pytest change above, please could you make the doctest one too. Thank you

I did this already 😀

Comment thread src/alphabet.cpp Outdated
:sig=(self: Alphabet, *, word: type) -> None:
::only-document-once:
Default constructor.
This function constructs an empty alphabet.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not a default constructor because it has a parameter.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@james-d-mitchell james-d-mitchell enabled auto-merge (rebase) June 19, 2026 08:19
@james-d-mitchell james-d-mitchell merged commit f0e78da into libsemigroups:main Jun 19, 2026
24 checks passed
@james-d-mitchell james-d-mitchell deleted the alphabet branch June 19, 2026 09:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

libsemigroups-feature-not-yet-supported Label for issues and PRs related to features of libsemigroups not yet supported here

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants